-
Notifications
You must be signed in to change notification settings - Fork 2.6k
feat: Add hierarchical rule loading from parent directories #4365
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
Cc: @swiftugandan, @yisrael-rosen, @MusiCode1 because they heart-emojii'ed this feature in Cline last year, but it was never merged. |
f8767b6 to
61a173a
Compare
|
This is ready for review. |
daniel-lxs
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @KJ7LNW, Thanks for your contribution! The implementation looks clean and the feature addresses a real pain point.
However I would like to know what you think about allowing users to disable this behavior, specially since for some users it might be quite unexpected or undesirable (it can potentially add a lot of tokens to each request without the user knowing).
Also the implementation could really benefit from some unit tests.
Let me know what you think!
src/core/prompts/sections/__tests__/custom-instructions.test.ts
Outdated
Show resolved
Hide resolved
61a173a to
368c1d3
Compare
|
The setting has been added on the "Modes" page: and [Roo Rules] Detected .roo/rules-code directory: /home/ewheeler/src/roo/Roo3/.roo/rules-code
[Roo Rules] Detected .roo/rules directory: /home/ewheeler/src/roo/Roo3/.roo/rules
[Roo Rules] Detected .roo/rules directory: /home/ewheeler/src/roo/.roo/rules
[Roo Rules] Detected .roo/rules-code directory: /home/ewheeler/.roo/rules-code
[Roo Rules] Detected .roo/rules directory: /home/ewheeler/.roo/rules |
|
Hey @KJ7LNW, it seems like the failing test is genuine. Can you take a look? |
174f320 to
3886e16
Compare
3886e16 to
d97ac5d
Compare
daniel-lxs
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not really sure how this PR is consistently triggering this test and causing it to fail.
I'll approve the changes but it would be great if @cte could take a quick look, maybe I'm missing something here.
|
Hey @KJ7LNW, I looked into the test failure and tried to sort it out. The issue seems to come from the webview build trying to bundle Node.js-only modules like Since I tried moving the call to While the tests are passing now, I still couldn’t get the actual behavior to work correctly again. Something's still off, and custom instructions aren't loading the way they should. Please take a look when you get a chance and let me know what you find. Happy to pair on it if needed. |
|
@daniel-lxs - I was able to reproduce it locally by running a build: The problem was caused by d97ac5d04a27897b7259c0e78d1f1d23e2aad79d - for some reason it does not like I am going to rework this pull request so leave it as in progress for now. |
This commit implements the parentRulesMaxDepth setting which controls how many parent directories are searched for .roo/rules files. The setting is accessible from the Modes view and persists across sessions. Key changes: - Added parentRulesMaxDepth to the global settings schema - Added UI component in ModesView.tsx with improved styling - Updated state management to properly persist the setting Signed-off-by: Eric Wheeler <[email protected]>
Signed-off-by: Eric Wheeler <[email protected]>
bda6ab9 to
58ac018
Compare
Add comprehensive test coverage for the parentRulesMaxDepth feature: - Mock ContextProxy for consistent test behavior - Test default depth behavior - Test error handling when ContextProxy is unavailable - Test traversing parent directories based on maxDepth - Test stopping at root directory - Test safety break handling - Add consistent os.homedir mock for custom-instructions tests Signed-off-by: Eric Wheeler <[email protected]>
58ac018 to
f5e39d1
Compare
Added translations for the parentRulesMaxDepth setting to all language files. This setting controls how many directory levels up to search for .roo/rules files, enabling hierarchical rule loading from parent directories. Signed-off-by: Eric Wheeler <[email protected]>
|
@daniel-lxs this is fixed up and ready to review |
|
@KJ7LNW Thanks for your work on this PR. However, we're closing it for now, as the solution introduces complexity that seems disproportionate to the relatively niche problem described in the linked issue. At this stage, we don't have sufficient evidence indicating this impacts enough users to justify the additional complexity and maintenance overhead. If further data emerges showing broader user impact, please feel free to reopen or revisit this approach. |

Context
Currently, Roo only loads custom rules from the immediate working directory, which creates several limitations:
This PR implements hierarchical loading of custom rules by traversing from the current working directory up to the root directory, collecting and merging rules at each level. This enables:
Implementation
Added a new
addSingleCustomInstructionsfunction to handle loading rules from a single directory level and modifiedaddCustomInstructionsto traverse from the current directory up to the root. Rules from parent directories are loaded first (starting from root).How to Test
.roo/rules/directory in a parent directory of your project.roo/rules/directory in your project directory with different rulesGet in Touch
Discord: KJ7LNW
Fixes #4364
Important
This PR adds hierarchical rule loading from parent directories with a configurable depth, enhancing rule management across project levels.
parentRulesMaxDepth).parentRulesMaxDepthtoglobal-settings.tsto control directory traversal depth.ModesView.tsxfor settingparentRulesMaxDepth.getRooDirectoriesForCwd()inindex.tsto support parent directory traversal.Setto avoid duplicate directories and ensures proper sorting.index.spec.tsto cover new traversal logic and configuration.parentRulesMaxDepthsetting.This description was created by
for 43d0bb0. You can customize this summary. It will automatically update as commits are pushed.